Skip to content

Fix XML Beautify indentation after xmlns self-closing element#2608

Open
alleria173 wants to merge 1 commit into
gchq:masterfrom
alleria173:fix/xml-beautify-xmlns-indentation
Open

Fix XML Beautify indentation after xmlns self-closing element#2608
alleria173 wants to merge 1 commit into
gchq:masterfrom
alleria173:fix/xml-beautify-xmlns-indentation

Conversation

@alleria173

@alleria173 alleria173 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Description
The XML Beautify operation produces incorrect indentation for any element that follows a self-closing tag with an xmlns attribute (e.g. <b xmlns="foo" />).

Root cause: vkbeautify's preprocessor splits <b xmlns="foo" /> into two fragments — <b and xmlns="foo" /> — by inserting a delimiter before the xmlns= token. The <b fragment matches the opening-element rule and increments the indentation depth, but the corresponding decrement never fires because the closing /> is on the xmlns fragment rather than a standalone <elm/> token. Every sibling after the self-closing element is therefore indented one level too deep.

The fix moves the xmlns branch before the generic /> branch in the if-else chain, and decrements depth when the xmlns fragment ends with /> (signalling a self-closing element). This restores the correct depth for all subsequent siblings.

Because modifying node_modules/vkbeautify/index.js directly would be overwritten by npm install, the relevant functions have been inlined into XMLBeautify.mjs with the fix applied and the vkbeautify import removed. createShiftArr adds 31 lines (lines 9–39) and the xml formatter xmlBeautify adds 69 lines (lines 41–109). Both carry a JSDoc attribution to the original library: Adapted from vkBeautify (c) 2012 Vadim Kiryukhin, MIT/GPL dual licence.

Existing Issue
Fixes #2501 #2501

Screenshots
N/A — no visual changes.

AI disclosure
Claude Code (claude.ai/code) was used to assist in diagnosing the root cause, implementing the fix, and generating test cases. All code has been reviewed and understood by the submitter.

Test Coverage
A new test file tests/operations/tests/XMLBeautify.mjs has been added with 8 test cases covering baseline behaviour, the two xmlns regression scenarios from the issue, xmlns on non-self-closing tags, multiple xmlns attributes on a single self-closing element, correct indentation of siblings after the fix, XML declarations, and configurable indent strings. All 2064 operation tests pass (npm test).

vkbeautify splits `<b xmlns="foo" />` into two fragments during
preprocessing. The `<b` fragment incorrectly incremented the indent
depth, but the corresponding decrement never fired because `/>` was
on the xmlns fragment, not a standalone self-closing token. Subsequent
siblings were therefore indented one level too deep.

Fix by moving the xmlns branch before the generic `/>` branch in the
if-else chain and decrementing depth when the xmlns fragment ends
with `/>`. The vkbeautify xml and createShiftArr functions are inlined
into XMLBeautify.mjs to avoid patching node_modules directly.

Fixes gchq#2501

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XML Beautify: indentation screwed by xmlns attribute in self-closing element

1 participant